Skip to content
This repository was archived by the owner on Oct 25, 2021. It is now read-only.

Regina_Gabdrakhimova#21

Open
GabdrakhimovaRR wants to merge 30 commits intomasterfrom
feature/ReginaGabdrakhimova
Open

Regina_Gabdrakhimova#21
GabdrakhimovaRR wants to merge 30 commits intomasterfrom
feature/ReginaGabdrakhimova

Conversation

@GabdrakhimovaRR
Copy link
Collaborator

Completed homework_1

@NikolaevArtem NikolaevArtem added wontfix bug Code fix needed and removed wontfix labels Jul 9, 2021
@GabdrakhimovaRR GabdrakhimovaRR added the readyForReview Sign for Artem to take a look label Jul 10, 2021
@NikolaevArtem NikolaevArtem added HW_1 Good to go style Style fix needed and removed bug Code fix needed readyForReview Sign for Artem to take a look labels Jul 11, 2021
@NikolaevArtem NikolaevArtem changed the title feature/ReginaGabdrakhimova Regina_Gabdrakhimova Jul 11, 2021
@GabdrakhimovaRR GabdrakhimovaRR removed the style Style fix needed label Jul 26, 2021
@GabdrakhimovaRR GabdrakhimovaRR added the readyForReview Sign for Artem to take a look label Jul 26, 2021
@GabdrakhimovaRR GabdrakhimovaRR removed the readyForReview Sign for Artem to take a look label Aug 23, 2021
@NikolaevArtem
Copy link
Owner

Seems forgotten

@NikolaevArtem NikolaevArtem removed the readyForReview Sign for Artem to take a look label Sep 22, 2021
@@ -0,0 +1,7 @@
package course_project;

public class Main {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite a strange ship here
image

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! The game is playable and has a nice description, the code is pretty readable. It seems there are some bugs, good that you have automatic ship placement, the interface is cool. You show knowledge of Java Core and Java libraries. Abstractions and architecture is TBD

To make it better: add some features (like automatically mark cells around destroyed ship as miss), introduce the architecture, and more abstractions.

Ping me when you resolved this bug and tested your app manually for other bugs (like, what happens if the computer hits a cell already marked as a miss?)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The game shows me the opponent's field in the beginning? Have you played your game yourself?)
image

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opt: Also, there are no messages what happened - did computer (or me) hit or miss, how many times computer shot and etc...

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved!

Comment on lines 64 to 75
public MyImmutableBag changeCash(int newCash) {
return new MyImmutableBag(newCash, documents, pills);
}

public MyImmutableBag changeDocuments(ArrayList<String> newDocuments) {
return new MyImmutableBag(cash, newDocuments, pills);
}

public MyImmutableBag changePills(Map<String, String> newPills) {
return new MyImmutableBag(cash, documents, newPills);
}
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you don't make a copy of mutable objects, thus your class is mutable


public class CustomRegexMatcher {

public void run() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice example with different messages!

Comment on lines 37 to 39
public int hashCode() {
return new Random().nextInt(121);
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's unclear from the homework description, but here is not a mutable key problem, it's just invalid hashcode impl. Mutable key problem appears when hashcode generation is valid

@@ -0,0 +1,24 @@
package homework_7;

public class Main {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, great example with type conversion!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any issues in homework 7? Label has not been tagged

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues, added

Copy link
Owner

@NikolaevArtem NikolaevArtem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! The code is readable, covered by tests, I see you experimented with most of what we've learned in lectures. There are some minor issues to fix and a bug in Cource project TBD

@NikolaevArtem NikolaevArtem added the bug Code fix needed label Sep 22, 2021
@GabdrakhimovaRR GabdrakhimovaRR added readyForReview Sign for Artem to take a look and removed bug Code fix needed labels Sep 23, 2021
@NikolaevArtem NikolaevArtem added Course Project and removed readyForReview Sign for Artem to take a look labels Sep 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants